Core 1389 toc button accessibility issues #2615
Conversation
RoyEJohnson
force-pushed
the
core-1389-toc-button-accessibility-issues
branch
from
26a927b to
daac86a
Compare
There was a problem hiding this comment.
I made a directory and split this file into pieces
| > svg { | ||
| ${toolbarIconStyles}; | ||
| } | ||
|
|
There was a problem hiding this comment.
There's no more hidden version of this in desktop. I also refactored two mobile functions into one.
There was a problem hiding this comment.
Everything here is moved here unchanged.
|
|
||
|
|
||
| // tslint:disable-next-line: variable-name | ||
| export const TOCControlButton = tocConnector(({open, close, isOpen, ...props}: MiddleProps) => { |
There was a problem hiding this comment.
This is the new button that replaces two buttons. Added aria-controls
Creates a unified TOCControlButton component that maintains focus when toggling the table of contents state. This fixes the accessibility issue where focus was lost when the button state changed. Changes: - Added TOCControlButton component with aria-expanded and aria-controls - Replaced OpenTOCControl and CloseTOCControl with single TOCControlButton - Button now uses simplified "Table of Contents" label - Button maintains focus across state changes 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
RoyEJohnson
force-pushed
the
core-1389-toc-button-accessibility-issues
branch
from
842b475 to
e2977d6
Compare
There was a problem hiding this comment.
I tested on Heroku and the label is there and the button seems to keep focus, but there's one small issue where the ToC starts open, but the button starts on the closed state, maybe related to isOpen always being set to null? This is probably the only thing that I'd say really needs to be fixed.
The video in the slack thread also mentions a focus order issue that is not on the card, but it's possible that is just a "won't fix".
| aria-controls='toc-sidebar' | ||
| aria-label={`Click to ${isOpen ? 'close' : 'open'} the Table of Contents`} | ||
| onClick={isOpen ? close : open} | ||
| isOpen={null} |
There was a problem hiding this comment.
Any particular reason why this attribute gets null and not isOpen?
| message={isOpen ? openSearchMessage : closedSearchMessage} | ||
| data-analytics-label={isOpen ? 'Click to close Search' : 'Click to open Search'} | ||
| onClick={(desktop && hasQuery) || isOpen ? close : open} | ||
| isOpen={desktop ? hasQuery || props.isOpen : props.isOpen} |
There was a problem hiding this comment.
I guess you just copied this but this logic is pretty confusing, especially with isOpen vs props.isOpen
| import type { InnerProps } from './types'; | ||
| import styled, { css } from 'styled-components/macro'; | ||
| import { toolbarIconColor } from '../constants'; | ||
| import { toolbarDefaultButton, toolbarDefaultText } from '../Toolbar/styled'; |
There was a problem hiding this comment.
These could be sorted I suppose
CORE-1389